Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CS12 Support #117

Merged
merged 16 commits into from
Aug 12, 2015
Merged

CS12 Support #117

merged 16 commits into from
Aug 12, 2015

Conversation

marcparadise
Copy link
Member

This PR brings chef-zero behaviors in line with CS12 as of 12.0.5

It also removes support for chef-pedant, because the repositories oc-chef-pedant and chef-pedant are being merged.

Some of these changes (specifically around admin and private key flags in clients)are not backwards compatible with OSC11 behaviors.

@marcparadise
Copy link
Member Author

Some minor cleanup still planned, this is just a preliminary PR.

@tyler-ball
Copy link
Contributor

\cc @lamont-granquist @danielsdeleo @stevendanna @jtimberman per the Chef maintainers file

@marcparadise
Copy link
Member Author

Oops, thanks @tyler-ball

@tyler-ball
Copy link
Contributor

Also @jkeiser

@marcparadise
Copy link
Member Author

I would like to get this merged in as soon as is reasonably possible - at this point the merged oc-chef-pedant is on master, and chef-pedant has been deprecated. While current master of chef-zero will continue to work as-is because chef-pedant is pinned to a version, all new tests will be getting added to oc-chef-pedant.

@jkeiser
Copy link
Contributor

jkeiser commented Mar 10, 2015

Note I am out this week.

@@ -20,29 +20,7 @@ def delete(request)
json_response(200, ChefData::DataNormalizer.normalize_user(user, request.rest_path[3], ['username'], server.options[:osc_compat]))
end

def post(request)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than removing this I think we need to move it and slightly change this code. The actual endpoint that does this is:

POST /organizations/ORGNAME/users

with a body of {"username": "USERTOADD"}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the Chef Server support both endpoints? POST /organizations/ORGNAME/users and POST /organizations/ORGNAME/users/USER? If we want chef-zero to emulated CS12 I would think we want want it to do whatever CS12 does. I'm not sure when the endpoint was changed so I don't know when the old one was deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current chef server only support POST /organization/ORGNAME/users/. I don't think there is a version that supports POST /organizations/ORGNAME/users/USER but I haven't look extensively.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved this in andrewjamesbrown@0447018 which is part of #146

@@ -65,7 +65,13 @@ def put(request)
else
response = FFI_Yajl::Parser.parse(result[2], :create_additions => false)
end
response['private_key'] = private_key if private_key

if request.rest_path[2] == 'clients'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super confusing branching logic - I would expect to see actor_endpoint_spec.rb updated. Unless all the testing is through pedant?

@tyler-ball
Copy link
Contributor

Just as side note - if this removes CS11 functionality this should be released as a major version update. And it looks like you're doing that as I just saw in the Changelog :) Thanks!

@@ -3,7 +3,7 @@ gemspec

gem 'rest-client', :github => 'opscode/rest-client'

gem 'chef-pedant', :github => 'opscode/chef-pedant', :tag => '1.0.46'
gem 'oc-chef-pedant', :github => 'opscode/oc-chef-pedant', :tag => '2.0.0'

gem 'chef', :github => 'opscode/chef', :tag => '12.1.0.rc.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be updated to 12.3.0 (or at least not the RC version)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in #146

@tyler-ball
Copy link
Contributor

Is this stalled? We would like to see it merged so we could get chef/cheffish#50 merged.

@andrewjamesbrown
Copy link

@tyler-ball, hopefully I've addressed the concerns. I'm also keen to see chef/cheffish#50 merged. Thanks!

Andrew Brown added 2 commits July 10, 2015 10:25
This commit re-adds support for POST /organizations/ORGNAME/users.
Since the code is very similar to
/organizations/ORG/association_requests, we introduced a helper module
to avoid code duplication.
@tyler-ball
Copy link
Contributor

I just did a rebase on master and force-pushed

@tyler-ball
Copy link
Contributor

Poking this - I think the only task left to do is update the gemspecs to depend on pedant from the chef-server repo, then test against that. @marcparadise do you agree?

@marcparadise
Copy link
Member Author

Yes, looks like it.
On Jul 20, 2015 4:36 PM, "Tyler Ball" [email protected] wrote:

Poking this - I think the only task left to do is update the gemspecs to
depend on pedant from the chef-server repo, then test against that.
@marcparadise https://github.com/marcparadise do you agree?


Reply to this email directly or view it on GitHub
#117 (comment).

Use the embedded pedant in the chef-server repo.  This also requires
slight changes to the pedant configuration, as the options have changed.
@andrewjamesbrown
Copy link

I've updated the gemspecs in #151. Note that there are a number of failed tests now: 1366 examples, 130 failures, 19 pending

@andrewjamesbrown
Copy link

@tyler-ball, I think the remaining task is complete, but I'm a little concerned that there are now some failed tests with Chef12. Please let me know if there is anything else I can help with.

@tyler-ball
Copy link
Contributor

@andrewjamesbrown Unfortunately it means that chef-zero probably needs to be updated to bring its behavior in line with CS 12 still. @marcparadise do you have time to look at the failures?

Andrew Brown added 2 commits August 12, 2015 08:46
Chef Zero does not yet 100% support CS12, hence we need to disable
several of the failing tests until the code is written.
Adding preliminary support for the _identifiers REST endpoint.
@andrewjamesbrown
Copy link

@tyler-ball, @marcparadise I've submitted #152 that makes the build pass. I've removed several of the failing tests from the pedant config, and added a new _identifiers endpoint.

@marcparadise
Copy link
Member Author

@tyler-ball unfortunately I'm tied up with other projects and DoD Pittsburgh for the remainder of the week. @andrewjamesbrown thanks for the PR to get past the errors!

@marcparadise
Copy link
Member Author

I've opened issue #154 to cover server API versioning behaviors.

tyler-ball added a commit that referenced this pull request Aug 12, 2015
@tyler-ball tyler-ball merged commit 3116c8c into master Aug 12, 2015
@tyler-ball tyler-ball deleted the mp/merge-pedants branch August 12, 2015 18:12
@thommay thommay added Type: Enhancement Adds new functionality. and removed enhancement labels Jan 24, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants